Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding applications worflow #1

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

neckron
Copy link

@neckron neckron commented Mar 22, 2023

Adding applications workflow for getting applications statistic into elk

FranRodriguezRojas and others added 3 commits March 21, 2023 19:25
Adding applications workflow for getting applications statistic into elk
@neckron neckron marked this pull request as ready for review March 22, 2023 17:00
README.md Outdated

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neckron

You should look over this README in more detail and remove anything not relevant.

For instance there is a section that lays out all the different type of loader options but you have listed here that it is only going to load into ELK.

There is another section that lists two options of authentication supported but you have mentioned that only one of those 'connected-apps' will work.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neckron Is there a reason it only loads into ELK? Is there not a common loading process already in the metrics toolkit we could leverage since it supports multiple? We might only use ELK but no reason not to use a common process if it exists.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neckron Is there a reason it only loads into ELK? Is there not a common loading process already in the metrics toolkit we could leverage since it supports multiple? We might only use ELK but no reason not to use a common process if it exists.

Yes Adam, there's is a common process (loader-router.xml) that uses a Choice-Router component to route to each target system loader flow. The problem here is at the beginning of each one of those load flows it's calling the Aggregate Platform Metrics flow reference. It makes difficult to reuse specific target system load logic. Would be great to just receiving the payload in each load flow but it's not designed in that way.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neckron

You should look over this README in more detail and remove anything not relevant.

For instance there is a section that lays out all the different type of loader options but you have listed here that it is only going to load into ELK.

There is another section that lists two options of authentication supported but you have mentioned that only one of those 'connected-apps' will work.

  • As there exists a difficulty to reuse loader-router.xml I'm trying to make clear that it will only loads in ELK. Ideally it would be nice to have it loading in the other targets but it requires an extra effort to do some kind of refactoring/adapting code update to the existing code.
  • Honestly I only tested it using connected-apps credentials. Let me do some tests with client credentials to make sure if works correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants